Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unneeded camera follow offset checks #857

Merged
merged 1 commit into from
Jun 10, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Jun 10, 2021

Signed-off-by: Ian Chen [email protected]

🦟 Bug fix

Summary

When handling mouse movements in camera follow mode, , there is a check to see if camera has actually moved or not. If it has moved, the camera offset is marked as dirty. However, I found that this check always passes because a variable (camWorldPos) is never set. It also looks like the check is just unnecessary and we are just doing additional calculations so I just removed the check.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 requested a review from jennuine June 10, 2021 03:04
@iche033 iche033 requested a review from chapulina as a code owner June 10, 2021 03:04
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Jun 10, 2021
@@ -1644,10 +1644,6 @@ void IgnRenderer::HandleMouseViewControl()
<< std::endl;
}

math::Vector3d camWorldPos;
if (!this->dataPtr->followTarget.empty())
this->dataPtr->camera->WorldPosition();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bug is likely here. This should probably have been:

math::Vector3d camWorldPos;
if (!this->dataPtr->followTarget.empty())
  camWorldPos = this->dataPtr->camera->WorldPosition();

@codecov
Copy link

codecov bot commented Jun 10, 2021

Codecov Report

Merging #857 (40bc4fa) into ign-gazebo4 (657ca6d) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo4     #857      +/-   ##
===============================================
+ Coverage        65.70%   65.72%   +0.01%     
===============================================
  Files              240      240              
  Lines            17788    17783       -5     
===============================================
  Hits             11687    11687              
+ Misses            6101     6096       -5     
Impacted Files Coverage Δ
src/gui/plugins/scene3d/Scene3D.cc 9.47% <0.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 657ca6d...40bc4fa. Read the comment docs.

@iche033 iche033 merged commit 5694336 into ign-gazebo4 Jun 10, 2021
@iche033 iche033 deleted the iche033/follow_offset_check branch June 10, 2021 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants